-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add GDPR parameters in ad request #2522
Conversation
Conflicts: src/adapters/stickyadstv.js test/spec/adapters/stickyadstv_spec.js
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see comments. also, should add tests.
modules/freewheelSSPBidAdapter.js
Outdated
@@ -223,6 +223,20 @@ export const spec = { | |||
componentId: getComponentId(currentBidRequest.params.format) | |||
}; | |||
|
|||
if (typeof currentBidRequest.params.gdpr !== 'undefined') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this wasn't done properly. buildRequests(bidRequests, bidderRequest)
has a second bidderRequest
argument where you'll find the gdpr
params. please see the doc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also, more specifically, I believe gdpr_consent
could exist while gdpr
is undefined. So checking gdpr !== 'undefined'
is not a good way to do this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thx, I'll have a look.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes have been made, thx again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Saw some additional changes that should be reviewed.
modules/freewheelSSPBidAdapter.js
Outdated
|
||
var vastParams = currentBidRequest.params.vastUrlParams; | ||
if (typeof vastParams === 'object') { | ||
for (kye in vastParams) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems to be a small typo here; believe it should be key
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done, thx
modules/freewheelSSPBidAdapter.js
Outdated
requestParams._fw_gdpr_consent = bidderRequest.gdprConsent.consentString; | ||
|
||
if (typeof currentBidRequest.gdprConsent.gdprApplies === 'boolean') { | ||
requestParams._fw_gdpr = currentBidRequest.gdprConsent.gdprApplies; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should update currentBidRequest
to bidderRequest
in these two lines of code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
This pull request introduces 1 alert when merging 2a18f58 into 71503a7 - view on LGTM.com new alerts:
Comment posted by LGTM.com |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@guillaume-sticky Thanks for making the changes; LGTM now.
modules/freewheelSSPBidAdapter.js
Outdated
|
||
var vastParams = currentBidRequest.params.vastUrlParams; | ||
if (typeof vastParams === 'object') { | ||
for (key in vastParams) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update - I thought this was addressed in the commit, but it seems it wasn't. The LGTM check noted this key
variable isn't properly declared (missing a let
). Could you update this spot?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
@guillaume-sticky Please submit a docs PR in the docs repo and make an update in your bidders page to add the following variable.
This can go directly below the variable for your 1.0 compliance. This will allow your adapter to appear in a table that shows GDPR compliant adapters. Thanks. |
will do. Thx! |
* add stickyadsTV bidder adapter * init unit test file * ad some unit tests * fix unit test on ad format with parameters * add some unit tests * add unit tests on getBid method * add some test cases in unit tests * minor fix on component id tag. * remove adapters-sticky.json test file * use top most accessible window instead of window.top * Pass in the bid request in the createBid call. * use top most accessible window instead of window.top * add unit tests * update unit tests * fix unit test. * fix CI build * add alias freewheel-ssp * update unit tests on bidderCode value * fix component id values and add unit tests * allws to use any outstream format. * fix ASLoader on futur outstream format versions * minor: fix code format. * update unit tests * minor fix code format * minor: add missing new line at eof * replace StickyAdsTVAdapter by freewheel ssp bd adapter (for prebid 1.0) * remove old stickyadstv unittest spec. * fix server response parsing if sent as object with 'body' field * use the vastXml field for video mediatype * add user sync pixel in freewheel ssp adapter * remove all console log calls (replaced using util helper) * remove useless bidderCode (automatically added by the bidderFactory) * Return the SYNC pixel to be added in the page by Prebid.js * remove instance level properties to enable concurrent bids with the same adapter instance. * fix the request apss through and corresponding unit tests * fix 'freeheelssp' typo * add vast parameters feature and GDPR params in VAST request * fix lint issues * add gdpr parameter support on freewheelSSPBidAdapter * use bidderrequest to read gdpr parameters and update unit tests * fix lint errors * fix lint errors * fix typo and bidderRequest reference. * fix bidderRequest reference. * add missing declaration for 'key' variable
* add stickyadsTV bidder adapter * init unit test file * ad some unit tests * fix unit test on ad format with parameters * add some unit tests * add unit tests on getBid method * add some test cases in unit tests * minor fix on component id tag. * remove adapters-sticky.json test file * use top most accessible window instead of window.top * Pass in the bid request in the createBid call. * use top most accessible window instead of window.top * add unit tests * update unit tests * fix unit test. * fix CI build * add alias freewheel-ssp * update unit tests on bidderCode value * fix component id values and add unit tests * allws to use any outstream format. * fix ASLoader on futur outstream format versions * minor: fix code format. * update unit tests * minor fix code format * minor: add missing new line at eof * replace StickyAdsTVAdapter by freewheel ssp bd adapter (for prebid 1.0) * remove old stickyadstv unittest spec. * fix server response parsing if sent as object with 'body' field * use the vastXml field for video mediatype * add user sync pixel in freewheel ssp adapter * remove all console log calls (replaced using util helper) * remove useless bidderCode (automatically added by the bidderFactory) * Return the SYNC pixel to be added in the page by Prebid.js * remove instance level properties to enable concurrent bids with the same adapter instance. * fix the request apss through and corresponding unit tests * fix 'freeheelssp' typo * add vast parameters feature and GDPR params in VAST request * fix lint issues * add gdpr parameter support on freewheelSSPBidAdapter * use bidderrequest to read gdpr parameters and update unit tests * fix lint errors * fix lint errors * fix typo and bidderRequest reference. * fix bidderRequest reference. * add missing declaration for 'key' variable
* add stickyadsTV bidder adapter * init unit test file * ad some unit tests * fix unit test on ad format with parameters * add some unit tests * add unit tests on getBid method * add some test cases in unit tests * minor fix on component id tag. * remove adapters-sticky.json test file * use top most accessible window instead of window.top * Pass in the bid request in the createBid call. * use top most accessible window instead of window.top * add unit tests * update unit tests * fix unit test. * fix CI build * add alias freewheel-ssp * update unit tests on bidderCode value * fix component id values and add unit tests * allws to use any outstream format. * fix ASLoader on futur outstream format versions * minor: fix code format. * update unit tests * minor fix code format * minor: add missing new line at eof * replace StickyAdsTVAdapter by freewheel ssp bd adapter (for prebid 1.0) * remove old stickyadstv unittest spec. * fix server response parsing if sent as object with 'body' field * use the vastXml field for video mediatype * add user sync pixel in freewheel ssp adapter * remove all console log calls (replaced using util helper) * remove useless bidderCode (automatically added by the bidderFactory) * Return the SYNC pixel to be added in the page by Prebid.js * remove instance level properties to enable concurrent bids with the same adapter instance. * fix the request apss through and corresponding unit tests * fix 'freeheelssp' typo * add vast parameters feature and GDPR params in VAST request * fix lint issues * add gdpr parameter support on freewheelSSPBidAdapter * use bidderrequest to read gdpr parameters and update unit tests * fix lint errors * fix lint errors * fix typo and bidderRequest reference. * fix bidderRequest reference. * add missing declaration for 'key' variable
* add stickyadsTV bidder adapter * init unit test file * ad some unit tests * fix unit test on ad format with parameters * add some unit tests * add unit tests on getBid method * add some test cases in unit tests * minor fix on component id tag. * remove adapters-sticky.json test file * use top most accessible window instead of window.top * Pass in the bid request in the createBid call. * use top most accessible window instead of window.top * add unit tests * update unit tests * fix unit test. * fix CI build * add alias freewheel-ssp * update unit tests on bidderCode value * fix component id values and add unit tests * allws to use any outstream format. * fix ASLoader on futur outstream format versions * minor: fix code format. * update unit tests * minor fix code format * minor: add missing new line at eof * replace StickyAdsTVAdapter by freewheel ssp bd adapter (for prebid 1.0) * remove old stickyadstv unittest spec. * fix server response parsing if sent as object with 'body' field * use the vastXml field for video mediatype * add user sync pixel in freewheel ssp adapter * remove all console log calls (replaced using util helper) * remove useless bidderCode (automatically added by the bidderFactory) * Return the SYNC pixel to be added in the page by Prebid.js * remove instance level properties to enable concurrent bids with the same adapter instance. * fix the request apss through and corresponding unit tests * fix 'freeheelssp' typo * add vast parameters feature and GDPR params in VAST request * fix lint issues * add gdpr parameter support on freewheelSSPBidAdapter * use bidderrequest to read gdpr parameters and update unit tests * fix lint errors * fix lint errors * fix typo and bidderRequest reference. * fix bidderRequest reference. * add missing declaration for 'key' variable
* add stickyadsTV bidder adapter * init unit test file * ad some unit tests * fix unit test on ad format with parameters * add some unit tests * add unit tests on getBid method * add some test cases in unit tests * minor fix on component id tag. * remove adapters-sticky.json test file * use top most accessible window instead of window.top * Pass in the bid request in the createBid call. * use top most accessible window instead of window.top * add unit tests * update unit tests * fix unit test. * fix CI build * add alias freewheel-ssp * update unit tests on bidderCode value * fix component id values and add unit tests * allws to use any outstream format. * fix ASLoader on futur outstream format versions * minor: fix code format. * update unit tests * minor fix code format * minor: add missing new line at eof * replace StickyAdsTVAdapter by freewheel ssp bd adapter (for prebid 1.0) * remove old stickyadstv unittest spec. * fix server response parsing if sent as object with 'body' field * use the vastXml field for video mediatype * add user sync pixel in freewheel ssp adapter * remove all console log calls (replaced using util helper) * remove useless bidderCode (automatically added by the bidderFactory) * Return the SYNC pixel to be added in the page by Prebid.js * remove instance level properties to enable concurrent bids with the same adapter instance. * fix the request apss through and corresponding unit tests * fix 'freeheelssp' typo * add vast parameters feature and GDPR params in VAST request * fix lint issues * add gdpr parameter support on freewheelSSPBidAdapter * use bidderrequest to read gdpr parameters and update unit tests * fix lint errors * fix lint errors * fix typo and bidderRequest reference. * fix bidderRequest reference. * add missing declaration for 'key' variable
* add stickyadsTV bidder adapter * init unit test file * ad some unit tests * fix unit test on ad format with parameters * add some unit tests * add unit tests on getBid method * add some test cases in unit tests * minor fix on component id tag. * remove adapters-sticky.json test file * use top most accessible window instead of window.top * Pass in the bid request in the createBid call. * use top most accessible window instead of window.top * add unit tests * update unit tests * fix unit test. * fix CI build * add alias freewheel-ssp * update unit tests on bidderCode value * fix component id values and add unit tests * allws to use any outstream format. * fix ASLoader on futur outstream format versions * minor: fix code format. * update unit tests * minor fix code format * minor: add missing new line at eof * replace StickyAdsTVAdapter by freewheel ssp bd adapter (for prebid 1.0) * remove old stickyadstv unittest spec. * fix server response parsing if sent as object with 'body' field * use the vastXml field for video mediatype * add user sync pixel in freewheel ssp adapter * remove all console log calls (replaced using util helper) * remove useless bidderCode (automatically added by the bidderFactory) * Return the SYNC pixel to be added in the page by Prebid.js * remove instance level properties to enable concurrent bids with the same adapter instance. * fix the request apss through and corresponding unit tests * fix 'freeheelssp' typo * add vast parameters feature and GDPR params in VAST request * fix lint issues * add gdpr parameter support on freewheelSSPBidAdapter * use bidderrequest to read gdpr parameters and update unit tests * fix lint errors * fix lint errors * fix typo and bidderRequest reference. * fix bidderRequest reference. * add missing declaration for 'key' variable
Type of change
Description of change
Add some parameters support for freewheel-ssp bidder adapter.
Especially GDPR parameters to be transmit to our ad server
contact email of the adapter’s maintainer
gandouard@freewheel.tv
[ *] official adapter submission